-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lucas Kim's bootcamp #102
Lucas Kim's bootcamp #102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed
relative_y = self.waypoint.location_y - report.position.location_y | ||
|
||
# Check if the waypoint has already been reached | ||
if abs(relative_x) < allowed_error and abs(relative_y) < allowed_error: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, we use Euclidean distance (L2 norm) rather than taxicab/manhattan distance (L1 norm). Try to use that instead
@@ -4,6 +4,8 @@ | |||
Travel to designated waypoint and then land at a nearby landing pad. | |||
""" | |||
|
|||
import math |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this bootcamp, you are not allowed to import any libraries. The bootcamp can be completed without importing math or using square roots. You can think about what is your purpose for calculating distance
# Create BoundingBox object and append to list | ||
_, box = bounding_box.BoundingBox.create(xyxy_box) | ||
|
||
if box is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a "success" indicator returned from the create function, you can use that instead. It's more readable and can handle more cases than box does not exist. Maybe the box exists but is malformed.
# Do something based on the report and the state of this class... | ||
if report.status == drone_status.DroneStatus.HALTED: | ||
# Check if the waypoint has already been reached | ||
if self.started_moving_to_landing_pad: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although in this bootcamp, you are guarantee to make it to your destination, sometimes due to wind you end up somewhere else. Please use the acceptance_radius just like you used the "error" from the other task.
# Check if the waypoint has already been reached | ||
if self.started_moving_to_landing_pad: | ||
command = commands.Command.create_land_command() | ||
elif self.started_moving_to_waypoint: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, please confirm that you are indeed within the acceptance radius of the waypoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed
|
||
# Check if the landing pad or waypoint has already been reached | ||
if ( | ||
self.__squared_distance(report.position, closest_landing_pad) < allowed_error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this, the function run()
has a parameter acceptance_radius
which you should use. The "error" should thus be squared for this case.
and self.has_reached_waypoint | ||
): | ||
command = commands.Command.create_land_command() | ||
elif self.__squared_distance(report.position, self.waypoint) < allowed_error: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
closest_landing_pad = min( | ||
landing_pad_locations, | ||
key=lambda pad: self.__squared_distance(report.position, pad), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you moved it outside of the conditions. It should probably be after line 84, when you have reached the waypoint.
# Create BoundingBox object and append to list | ||
success, box = bounding_box.BoundingBox.create(xyxy_box) | ||
|
||
if not success: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In statistics, we normally discard the entire data point (the whole image) if part of it has gone wrong (a bounding box).
(You do not need to change anything, just something to think about)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
No description provided.